Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update action-diff to not include the stable component ID #435

Merged
merged 7 commits into from
Jan 4, 2024
Merged

Update action-diff to not include the stable component ID #435

merged 7 commits into from
Jan 4, 2024

Conversation

cBournhonesque
Copy link
Contributor

@cBournhonesque cBournhonesque commented Jan 4, 2024 •
edited
Loading

This PR reworks how networking via ActionDiff works:

  • instead of using a "Stable ID Component" that uniquely identifies entities across client and server, we introduce the ActionDiffEvent type, which has:
    • an owner: the Entity that generates the action, or None if we are using a global ActionState
    • the action_diff itself
  • we remove the process_action_diffs system in favor of a method ActionState.apply_diff(action_diff), to leave more flexibility to users over how to process the action-diffs (perform entity-mapping, etc.)

I'm slightly confused about this: https://github.com/Leafwing-Studios/leafwing-input-manager/blob/main/src/systems.rs#L216: we detect if the button is Pressed/ValueChanged just based on the value?
What if we have a 'Value' type button that has a value of 1.0? We would then get a Pressed ActionDiff, which seems incompatible?

NOTE: it might actually be more efficient to have the ActionDiffEvent contains a Vec<ActionDiff> since we can compute all of them in one go for a given entity

alice-i-cecile
alice-i-cecile reviewed Jan 4, 2024
View reviewed changes
src/action_state.rs Outdated Show resolved Hide resolved
@alice-i-cecile
Copy link
Contributor

I'm slightly confused about this: https://github.com/Leafwing-Studios/leafwing-input-manager/blob/main/src/systems.rs#L216: we detect if the button is Pressed/ValueChanged just based on the value?
What if we have a 'Value' type button that has a value of 1.0? We would then get a Pressed ActionDiff, which seems incompatible?

Yeah, this seems a bit sloppy to me too. Can you make an issue about this?

NOTE: it might actually be more efficient to have the ActionDiffEvent contains a Vec since we can compute all of them in one go for a given entity

Agreed, I think this is important given the networking-centric nature of this code. Feel free to make that change in this PR or a follow-up.

@alice-i-cecile alice-i-cecile added the usability Reduce user friction label Jan 4, 2024
@cBournhonesque
Copy link
Contributor Author

I'm slightly confused about this: https://github.com/Leafwing-Studios/leafwing-input-manager/blob/main/src/systems.rs#L216: we detect if the button is Pressed/ValueChanged just based on the value?
What if we have a 'Value' type button that has a value of 1.0? We would then get a Pressed ActionDiff, which seems incompatible?

Yeah, this seems a bit sloppy to me too. Can you make an issue about this?

NOTE: it might actually be more efficient to have the ActionDiffEvent contains a Vec since we can compute all of them in one go for a given entity

Agreed, I think this is important given the networking-centric nature of this code. Feel free to make that change in this PR or a follow-up.

I made the change for the Event to contain a Vec<ActionDiff>.
And sure i'll create an issue!

@alice-i-cecile alice-i-cecile merged commit 046e040 into Leafwing-Studios:main Jan 4, 2024
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile left review comments

Assignees
No one assigned
Labels
usability Reduce user friction
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cBournhonesque @alice-i-cecile @cbournhonesque-sc